Skip to content

fix: force VerifyServerDataDir fallback over TCP#3485

Closed
Bella-Giraffety wants to merge 1 commit intogastownhall:mainfrom
Bella-Giraffety:polecat/mayor-gs-fta-verify-tcp-fallback
Closed

fix: force VerifyServerDataDir fallback over TCP#3485
Bella-Giraffety wants to merge 1 commit intogastownhall:mainfrom
Bella-Giraffety:polecat/mayor-gs-fta-verify-tcp-fallback

Conversation

@Bella-Giraffety
Copy link
Copy Markdown

Summary

  • add a dedicated explicit-TCP dolt sql helper for verification paths
  • switch VerifyServerDataDir's fallback SHOW DATABASES query to that helper
  • preserve inherited DOLT_CLI_PASSWORD only for remote fallback checks when config omits a password
  • add focused tests for local and remote explicit-TCP command construction

Why

Builds on the explicit-TCP helper work in #3470 and the daemon-helper parity follow-up in #3482.

VerifyServerDataDir still had one fallback path that could issue SHOW DATABASES through the older local dolt sql behavior. That left imposter detection able to drift back into embedded-mode / autodiscovery behavior instead of querying the live shared server.

This PR keeps that remaining fallback path on the same explicit TCP client path without broadening into unrelated Dolt shellout cleanup.

Validation

  • GOTOOLCHAIN=auto go test ./internal/doltserver -run 'TestBuildDoltSQLTCPClientCmd_(Local|Remote|RemoteNoPasswordPreservesInheritedCredentials)$|TestBuildDoltSQLCmd.*|TestCheckServerReachable.*|TestIsRunning.*'

Notes

@github-actions github-actions bot added the status/needs-triage Inbox — we haven't looked at it yet label Apr 1, 2026
@Bella-Giraffety
Copy link
Copy Markdown
Author

Closing this as redundant after re-auditing the stack. The VerifyServerDataDir fallback bug is real, but keeping #3470 plus #3482 is the cleaner split to carry forward, and this PR now reads as an extra follow-up layer in the same TCP-helper chain rather than an independent must-merge fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/needs-triage Inbox — we haven't looked at it yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants